improve: enhance CV validator error messages with invalid values and valid options#864
Conversation
…valid options - Institution validator now shows invalid institution_id and list of valid options - Model validator shows invalid model_id with available options - Experiment validator displays invalid experiment_id/sub_experiment_id with valid choices - Model types validator indicates which types are invalid/missing and shows allowed types - Parent experiment validator clarifies which parent_experiment_id is invalid for the given experiment All error messages now include: - The actual invalid value provided - The list of valid/allowed options from CV This greatly improves debugging for users when validation fails. Fixes MetOffice#863
There was a problem hiding this comment.
Pull request overview
This PR improves controlled-vocabulary (CV) validation diagnostics by embedding the invalid user-provided values and listing the valid/allowed options, making request-validation failures more actionable for users.
Changes:
- Enriches institution/model/experiment/sub-experiment validation errors with invalid values and valid options.
- Enhances model-type validation errors to identify specific invalid and missing types.
- Improves parent-experiment validation errors with clearer context and valid options.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| valid_institutions = list(cv_config.get_collection('institution_id').keys()) | ||
| raise CVEntryError( | ||
| f'Unknown institution_id "{request.metadata.institution_id}". ' | ||
| f'Valid options: {valid_institutions}' | ||
| ) |
There was a problem hiding this comment.
The error message prints valid_institutions using the raw dict.keys() order and Python list repr. For readability and determinism (and to keep logs/CLI output predictable), consider sorting the options and formatting them as a comma-separated string; if the CV list can be large, consider truncating with a count (e.g., show first N + total).
| invalid_types = set(model_types) - set(allowed_model_types) | ||
| raise CVEntryError( | ||
| f'Invalid model type(s) {invalid_types}. ' | ||
| f'Allowed types for experiment_id "{request.metadata.experiment_id}": {allowed_model_types}' | ||
| ) |
There was a problem hiding this comment.
invalid_types is a set, so its string representation is unordered and includes Python-specific braces (e.g., "{'AER', 'CHEM'}"). To keep the message human-friendly and stable, format these as a sorted list (or a joined string) rather than embedding the set directly.
| raise CVEntryError( | ||
| f'Unknown institution_id "{request.metadata.institution_id}". ' | ||
| f'Valid options: {valid_institutions}' | ||
| ) |
There was a problem hiding this comment.
This PR’s purpose is improved error-message content, but the existing unit tests only assert that CVEntryError is raised and don’t verify the new message details (invalid value + valid options). Consider extending the validator tests to assert key substrings of the message so future refactors don’t regress these user-facing diagnostics.
| missing_types = set(required_model_types) - set(model_types) | ||
| raise CVEntryError( | ||
| f'Missing required model type(s) {missing_types} ' | ||
| f'for experiment_id "{request.metadata.experiment_id}". ' | ||
| f'Provided types: {model_types}' | ||
| ) |
There was a problem hiding this comment.
missing_types is a set, so the error message will be unordered and use Python set formatting. For a stable, user-friendly message, format missing types as a sorted list / comma-separated string (and consider similarly formatting model_types if you want consistent output).
|
Hi Darner Iszat Diaz Zapana (@DarnerDiaz), Thanks for the interest in our project, but we are not in a position to take contributions from outside of the core development team at this time. In order to accept your PR, which looks quite reasonable from a quick inspection, I would need to ensure that appropriate contributor agreements and policies were in place, along with access to platforms where we deploy and use CDDS, and we are currently focussing all our energies on being ready to deliver data to the CMIP7 Assessment Fast Track in the next couple of months. |
Summary
This PR improves CV validator error messages to provide clearer feedback when validation fails. Instead of generic error messages, users now see:
Changes
Enhanced error messages in
cvs/common/request/validations/cv_validators.py:Institution Validator
Model Validator
Experiment Validator
Model Types Validator
Parent Experiment Validator
Benefits
✅ Users get clear, actionable error messages
✅ Easier debugging when CV validation fails
✅ Shows valid options directly in error message
✅ Helps users understand what values are acceptable
Testing